Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes an issue where byAdding(.extraAttributes(_:)) removes previously assigned extraAttributes. #430

Merged
merged 2 commits into from
Jun 12, 2023
Merged

Fixes an issue where byAdding(.extraAttributes(_:)) removes previously assigned extraAttributes. #430

merged 2 commits into from
Jun 12, 2023

Conversation

stleamist
Copy link
Contributor

@stleamist stleamist commented Jun 9, 2023 •
edited
Loading

Issue

byAdding(stringStyle:) preserves previously assigned extraAttributes, but byAdding(_:) doesn't.

let style = StringStyle()
    .byAdding(stringStyle: StringStyle(.extraAttributes([.backgroundColor: UIColor.white])))
    .byAdding(stringStyle: StringStyle(.extraAttributes([.foregroundColor: UIColor.black])))

BONAssert(attributes: style.attributes, key: .backgroundColor, value: UIColor.white) // ✅
BONAssert(attributes: style.attributes, key: .foregroundColor, value: UIColor.black) // ✅
let style = StringStyle()
    .byAdding(.extraAttributes([.backgroundColor: UIColor.white]))
    .byAdding(.extraAttributes([.foregroundColor: UIColor.black])) // overwrites previous line

BONAssert(attributes: style.attributes, key: .backgroundColor, value: UIColor.white) // ❌
BONAssert(attributes: style.attributes, key: .foregroundColor, value: UIColor.black) // ✅

Solution

Fixed update(part:) method to use add(extraAttributes:) for .extraAttributes(_:), like byAdding(stringStyle:) does.

@ZevEisenberg
Copy link
Collaborator

Thank you! This is a great catch. Would you mind swapping UIColor for BONColor, a platform-agnostic alias that will allow the tests to run on macOS?

@stleamist
Copy link
Contributor Author

Okay! I’ll swap it right now :)

@stleamist stleamist force-pushed the bug/byadding-parts-extraattributes-overriding branch from 6c79a4c to 90e216b Compare June 9, 2023 13:48
@stleamist
Copy link
Contributor Author

Hi @ZevEisenberg,

Could you review this PR, please?

@ZevEisenberg ZevEisenberg merged commit becf396 into Rightpoint:master Jun 12, 2023
@ZevEisenberg
Copy link
Collaborator

Merged and released! Thanks for your contribution. It makes me happy to know BonMot has active users!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZevEisenberg ZevEisenberg ZevEisenberg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stleamist @ZevEisenberg